fix(policy): validate the applicable advisor candidate by johntmyers · Pull Request #2850 · NVIDIA/OpenShell

johntmyers · 2026-08-20T22:27:22Z

Summary

Fixes policy-advisor approval so the prover evaluates the exact effective-policy candidate that can actually be applied. Reviews are bound to live policy, provider, and non-secret credential inputs; changed inputs refresh the candidate and require fresh review, while unchanged candidates reuse the stored prover result.

Related Issue

Closes #2821

Changes

Canonicalize mechanistic endpoint proposals against the live effective policy, preserving existing L7 inspection contracts and treating provider rules as immutable inputs.
Validate the complete merge, provider composition, credential preflight, and L7 policy shape before approval; persist actionable application errors separately from prover results.
Persist current/candidate effective-policy hashes and an opaque review token, and require token-bound single and bulk approvals.
Recompute from one live provider snapshot at approval time; reuse the prover verdict when the candidate token is unchanged and require fresh review when it changes.
Reconcile pending chunks after successful policy writes so proposals already covered by the active policy leave the inbox.
Surface candidate state in the CLI, TUI, policy.local API, and Go SDK.
Document the flow and add a Docker E2E regression for expanding an existing inspected endpoint without downgrading it to L4.

Testing

Checklist

Follows Conventional Commits
Commits are signed off (DCO)
Architecture docs updated

github-actions · 2026-08-20T22:27:50Z

🌿 Preview your docs: https://nvidia-preview-pr-2850.docs.buildwithfern.com/openshell

johntmyers · 2026-08-20T22:33:12Z

gator-agent

Blocked

Head SHA: 2d72d0662cbe0ae70e80e7df3347d2e77cd44c51
Gator payload: 4

Gator is blocked because PR #2850 has merge conflicts with main.

Next action: @johntmyers, update the branch from current main, resolve the conflicts, and push the resolved head. Gator will run the independent code review after GitHub reports the PR mergeable.

johntmyers

gator-agent

PR Review Status

Validation: Project-valid implementation of accepted issue #2821, covering exact-candidate evaluation, actionable application failures, and preservation of inspected endpoint contracts.
Head SHA: 6b82f885654867d1b2fe391fff2610dfc35283b5
Base SHA: ef296806f52c03956a4bb7ce9a384105160f9c6a
Merge base SHA: ef296806f52c03956a4bb7ce9a384105160f9c6a
Patch ID: f04e290221220295af9e228daa89d7891c2992fe
Gator payload: 4
Review mode: initial
Previous reviewed SHA: none
Review budget exhausted: no
Maintainer decision required: no

Blocking findings:

GATOR-6b82f885-01: Review tokens and effective-policy hashes use non-canonical protobuf encodings for nested map fields, so unchanged supported policies can appear stale.
GATOR-6b82f885-02: Canonicalizing a one-port denial against a multi-port inspected endpoint can grant the new binary every port in that endpoint.

Carried findings:

None

Non-blocking suggestions:

None

Docs: Fern documentation and the related CLI skill/reference were updated for the user-visible review-token and application-error behavior.

Next state: gator:in-review

johntmyers · 2026-08-20T23:40:02Z

🏗️ build-from-issue-agent

Addressed both blocking Gator findings in d9d38a561:

Canonical review/effective-policy hashing now recursively sorts protobuf maps while preserving repeated-field order.
A one-port advisor observation can reuse inspection metadata but cannot inherit sibling ports from a multi-port endpoint; it remains a narrow sandbox overlay.
Added focused policy and gateway regressions for nested map ordering and multi-port binary isolation.

Verification:

cargo test -p openshell-policy — 233 passed
cargo test -p openshell-server grpc::policy::tests — 195 passed
cargo clippy -p openshell-policy -p openshell-server --all-targets -- -D warnings — passed
mise run pre-commit — passed after rebasing onto current main
mise run e2e:mechanistic-existing-endpoint — attempted twice; both runs stopped during sandbox provisioning with MainProcessExited before reaching policy evaluation. The gateway started healthy, and the equivalent in-process gateway regression passes.

johntmyers

gator-agent

PR Review Status

Validation: Project-valid implementation of accepted issue #2821, covering exact-candidate evaluation, actionable application failures, and preservation of inspected endpoint contracts.
Head SHA: d9d38a561944dd041342c004dbfe6ff5b3c8d8de
Base SHA: 6c34a3c6458f1c1852fb9a89bb89b778a99fa19b
Merge base SHA: 6c34a3c6458f1c1852fb9a89bb89b778a99fa19b
Patch ID: e623f3b05064d830132c4a6d3fe099bba96f4397
Gator payload: 4
Review mode: follow_up
Previous reviewed SHA: 6b82f885654867d1b2fe391fff2610dfc35283b5
Review budget exhausted: no
Maintainer decision required: no

Thanks @johntmyers. I checked your two remediation updates in the author-only delta: canonical serialization now covers the relevant nested protobuf maps while preserving repeated-field order, and advisor canonicalization narrows a copied inspection contract to the observed port before owner selection.

Blocking findings:

No blocking findings remain. GATOR-6b82f885-01 and GATOR-6b82f885-02 are resolved by this head.

Carried findings:

None.

Docs: Fern documentation and the related CLI skill/reference cover the user-visible review-token and application-error behavior.

Next state: gator:in-review pending required E2E dispatch; pipeline watch begins only after a current-head E2E workflow is queued, running, or complete.

github-actions · 2026-08-20T23:48:36Z

Label test:e2e applied for d9d38a5. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

zredlined · 2026-08-21T14:31:23Z

@johntmyers Fix looks good to me. Ran a 30-min session with agent auto-approver, checking for the failure condition. 8 ambiguous agent policy proposal attempts happened in the session, each rejected before review (vs failing after approval)

Build and validate the exact effective-policy candidate before approval, bind review to live policy/provider/credential inputs, and preserve inspected endpoint contracts during mechanistic expansion. Closes #2821 Signed-off-by: John Myers <johntmyers@users.noreply.github.com>

Serialize nested protobuf maps in stable key order for proposal review tokens and effective-policy hashes. Narrow reused multi-port endpoint contracts to the denied port so advisor proposals cannot widen binary access. Add regressions for both cases. Signed-off-by: John Myers <johntmyers@users.noreply.github.com>

Create the issue 2821 regression sandbox detached with a durable canonical main process so policy denial, approval, and hot-reload checks run before lifecycle exit. Signed-off-by: John Myers <johntmyers@users.noreply.github.com>

johntmyers · 2026-08-21T16:00:30Z

🏗️ build-from-issue-agent

CI Diagnosis and E2E Test Attestation

The three failures had one lifecycle/test mismatch:

OIDC sandbox creation expected attached process output.
Managed-workspace sandbox creation expected attached process output.
Operator-workspace sandbox creation expected attached process output.

Current main fixes those exact assertions in #2856 by using detached durable sandboxes, and #2857 fixes stale main-process exit handling. PR #2850 is now rebased onto both.

The issue-specific regression had the same one-shot-main-process assumption, so 8e77aef74 now creates its sandbox detached with sleep infinity.

Field	Value
Commit	`8e77aef74`
Command	`mise run e2e:mechanistic-existing-endpoint`
Gateway mode	Docker
Result	✅ Passed

Test summary

Initial curl request denied with HTTP 403.
Advisor chunk auto-approved with prover: no new findings.
REST/read-only endpoint contract preserved.
Policy hot-reloaded and the subsequent curl request succeeded.

Additional verification:

cargo test -p openshell-policy — 233 passed
cargo test -p openshell-server grpc::policy::tests — 195 passed
mise run pre-commit — passed

johntmyers

gator-agent

PR Review Status

Validation: Project-valid implementation of accepted issue #2821, covering exact-candidate evaluation, actionable application failures, and preservation of inspected endpoint contracts.
Head SHA: 8e77aef742289e49d008571081f0814efe7968e1
Base SHA: de4c1fecf564cd16854447ec9c08659e7b64061b
Merge base SHA: de4c1fecf564cd16854447ec9c08659e7b64061b
Patch ID: a706ae97aee1ad413ab12fbd3ceb531ac450a34c
Gator payload: 4
Review mode: follow_up
Previous reviewed SHA: d9d38a561944dd041342c004dbfe6ff5b3c8d8de
Review budget exhausted: no
Maintainer decision required: no

Thanks @johntmyers. I checked the latest author-only delta and confirmed that it changes the issue regression to create a detached sandbox with a durable canonical process before exercising denial, approval, and hot reload. The two earlier reviewed commits are patch-equivalent across the rebase, so the prior resolutions remain valid. I also considered @zredlined's maintainer report that repeated ambiguous proposals were rejected before review; it is consistent with the intended fail-closed behavior and does not introduce a new obligation.

Blocking findings:

No blocking findings remain. GATOR-6b82f885-01 and GATOR-6b82f885-02 remain resolved.

Carried findings:

None.

Docs: Fern documentation and the related CLI skill/reference cover the user-visible review-token and application-error behavior; the latest delta changes only E2E lifecycle setup.

Next state: gator:watch-pipeline; current-head Branch Checks and E2E workflows are running.

Signed-off-by: John Myers <johntmyers@users.noreply.github.com>

johntmyers · 2026-08-21T18:42:24Z

🏗️ build-from-issue-agent

Implemented the approve-all follow-up in 22e595cb1.

What changed

Bulk approval now validates reviewed chunks against one live input snapshot, stages compatible operations in request order, and writes the resulting policy once.
A conflict inside the batch skips only that chunk, leaves it pending with an application error, and allows compatible chunks to be applied together.
Live inputs are recomputed immediately before persistence. If the reviewed snapshot is stale, the full write is rejected, so no partial policy update is applied.
Unchanged proposal hashes reuse the cached prover evaluation instead of rerunning the prover.
Provider-derived rules remain validation inputs; they are never copied into or mutated as part of the sandbox policy write.
The TUI no longer labels every skipped chunk as security-flagged. It directs the user to review the remaining pending chunks.
Proto comments now document the one-snapshot/one-write contract and the broader meaning of chunks_skipped.

Regression coverage

Multiple independent chunks are approved in one policy revision.
Cached proposal evaluation is retained for unchanged inputs.
A later advisor_proposed / inspected-endpoint conflict is skipped without discarding the compatible prefix.
A stale reviewed snapshot applies none of the batch.
TUI skip feedback does not assume a security finding.

Verification

cargo test -p openshell-server grpc::policy::tests — 198 passed
cargo test -p openshell-tui --lib — 47 passed
cargo clippy -p openshell-server -p openshell-tui --all-targets -- -D warnings
mise run pre-commit
mise run test
mise run ci

johntmyers

gator-agent

PR Review Status

Validation: Project-valid implementation of accepted issue #2821, covering exact-candidate evaluation, actionable application failures, and preservation of inspected endpoint contracts.
Head SHA: 22e595cb1749f2494d4cac8c3f63fbe42f94db84
Base SHA: de4c1fecf564cd16854447ec9c08659e7b64061b
Merge base SHA: de4c1fecf564cd16854447ec9c08659e7b64061b
Patch ID: 748f0cdcd8a263791af8d06c47b699cfcba91eb1
Gator payload: 4
Review mode: critical_only
Previous reviewed SHA: 8e77aef742289e49d008571081f0814efe7968e1
Review budget exhausted: yes
Maintainer decision required: no

Thanks @johntmyers. I checked your bulk-approval update in the author-only delta, including the one-snapshot staging path, one-write persistence guard, stale-input rejection, provider-layer validation, per-chunk conflict handling, and TUI result wording. The critical-only convergence pass found no newly introduced Critical defect.

Blocking findings:

No blocking findings remain. GATOR-6b82f885-01 and GATOR-6b82f885-02 remain resolved.

Carried findings:

None.

Docs: Existing Fern documentation covers the user-visible review-token and application-error behavior; this delta updates the bulk RPC contract and TUI wording without introducing a new setup flow.

Next state: gator:watch-pipeline; current-head Branch Checks and Helm Lint are green, and the required E2E workflow is queued/running.

johntmyers · 2026-08-21T19:31:55Z

gator-agent

Monitoring Complete

Monitoring is complete because this PR has merged.

Head SHA: 22e595cb1749f2494d4cac8c3f63fbe42f94db84
Gator payload: 4

Final status: the PR merged while gator was monitoring the pipeline.

I removed the active gator:* label because there is nothing left for gator to monitor on this PR.

johntmyers requested review from a team, derekwaynecarr, mrunalp and sjenning as code owners August 20, 2026 22:27

johntmyers mentioned this pull request Aug 20, 2026

bug(policy): auto-approval leaves unmergeable proposals pending without an actionable reason #2821

Closed

7 tasks

johntmyers added the gator:blocked Gator is blocked by process or repository gates label Aug 20, 2026

johntmyers force-pushed the fix/2821-policy-candidate-review/johntmyers branch from 2d72d06 to 6b82f88 Compare August 20, 2026 22:42

johntmyers commented Aug 20, 2026

View reviewed changes

Comment thread crates/openshell-server/src/grpc/policy.rs Outdated

Comment thread crates/openshell-policy/src/merge.rs

johntmyers added gator:in-review Gator is reviewing or awaiting PR review feedback and removed gator:blocked Gator is blocked by process or repository gates labels Aug 20, 2026

johntmyers force-pushed the fix/2821-policy-candidate-review/johntmyers branch from 6b82f88 to d9d38a5 Compare August 20, 2026 23:39

johntmyers commented Aug 20, 2026

View reviewed changes

johntmyers added the test:e2e Requires end-to-end coverage label Aug 20, 2026

johntmyers added gator:blocked Gator is blocked by process or repository gates and removed gator:in-review Gator is reviewing or awaiting PR review feedback labels Aug 20, 2026

zredlined assigned zredlined and johntmyers and unassigned zredlined Aug 21, 2026

zredlined self-requested a review August 21, 2026 14:32

zredlined previously approved these changes Aug 21, 2026

View reviewed changes

johntmyers added gator:watch-pipeline Gator is monitoring PR CI/CD status and removed gator:blocked Gator is blocked by process or repository gates labels Aug 21, 2026

johntmyers added 3 commits August 21, 2026 08:44

test(e2e): keep advisor sandbox running

8e77aef

Create the issue 2821 regression sandbox detached with a durable canonical main process so policy denial, approval, and hot-reload checks run before lifecycle exit. Signed-off-by: John Myers <johntmyers@users.noreply.github.com>

johntmyers dismissed zredlined’s stale review via 8e77aef August 21, 2026 16:00

johntmyers force-pushed the fix/2821-policy-candidate-review/johntmyers branch from d9d38a5 to 8e77aef Compare August 21, 2026 16:00

johntmyers commented Aug 21, 2026

View reviewed changes

zredlined approved these changes Aug 21, 2026

View reviewed changes

zredlined self-requested a review August 21, 2026 17:04

zredlined previously approved these changes Aug 21, 2026

View reviewed changes

johntmyers enabled auto-merge August 21, 2026 17:09

johntmyers added gator:merge-ready and removed gator:approval-needed Gator completed review; maintainer approval needed labels Aug 21, 2026

fix(policy): apply reviewed draft batches atomically

22e595c

Signed-off-by: John Myers <johntmyers@users.noreply.github.com>

johntmyers dismissed zredlined’s stale review via 22e595c August 21, 2026 18:42

johntmyers commented Aug 21, 2026

View reviewed changes

johntmyers added gator:watch-pipeline Gator is monitoring PR CI/CD status and removed gator:merge-ready labels Aug 21, 2026

pimlock approved these changes Aug 21, 2026

View reviewed changes

johntmyers added this pull request to the merge queue Aug 21, 2026

Merged via the queue into main with commit 679fe4c Aug 21, 2026
65 of 67 checks passed

johntmyers deleted the fix/2821-policy-candidate-review/johntmyers branch August 21, 2026 19:20

johntmyers removed the gator:watch-pipeline Gator is monitoring PR CI/CD status label Aug 21, 2026

Uh oh!

Conversation

johntmyers commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Issue

Changes

Testing

Checklist

Uh oh!

github-actions Bot commented Aug 20, 2026

Uh oh!

johntmyers commented Aug 20, 2026

Blocked

Uh oh!

johntmyers left a comment

Choose a reason for hiding this comment

PR Review Status

Uh oh!

Uh oh!

Uh oh!

johntmyers commented Aug 20, 2026

Uh oh!

johntmyers left a comment

Choose a reason for hiding this comment

PR Review Status

Uh oh!

github-actions Bot commented Aug 20, 2026

Uh oh!

zredlined commented Aug 21, 2026

Uh oh!

johntmyers commented Aug 21, 2026

CI Diagnosis and E2E Test Attestation

Test summary

Uh oh!

johntmyers left a comment

Choose a reason for hiding this comment

PR Review Status

Uh oh!

johntmyers commented Aug 21, 2026

What changed

Regression coverage

Verification

Uh oh!

johntmyers left a comment

Choose a reason for hiding this comment

PR Review Status

Uh oh!

Uh oh!

johntmyers commented Aug 21, 2026

Monitoring Complete

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

johntmyers commented Aug 20, 2026 •

edited

Loading